Invert the binding order of InitializerExpressions and CreationExpressions #30805
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…pressions: - Push intializer binding into the respective creation expression bindings so that initializer binding takes place *after* the creation, ensuring any referenced out vars have already been inferred, preventing infinite loops - Add test to show behavior
jaredpar
reviewed
Oct 29, 2018
| } | ||
| } | ||
| "; | ||
| var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.DebugExe, parseOptions: TestOptions.Regular); |
There was a problem hiding this comment.
Use the CreateCompilation helper. It's the standard helper we should be using whenever possible.
jaredpar
reviewed
Oct 29, 2018
| @" | ||
| public class C | ||
| { | ||
| public int Number{ get; set; } |
jaredpar
reviewed
Oct 29, 2018
| @@ -34493,6 +34493,32 @@ public class C | |||
| "); | |||
| Assert.Null(model.GetOperation(node3).Parent); | |||
| } | |||
|
|
|||
| [Fact] | |||
| public void OutVarInConstructorUsedInInitializer() | |||
There was a problem hiding this comment.
Do we have a similar test for collection initializers?
There was a problem hiding this comment.
Added OutVarInConstructorUsedInCollectionInitializer
jaredpar
reviewed
Oct 29, 2018
| } | ||
| finally | ||
| { | ||
| analyzedArguments.Free(); | ||
| } | ||
| } | ||
|
|
||
| private BoundExpression MakeBadExpressionForObjectCreation(ObjectCreationExpressionSyntax node, TypeSymbol type, BoundExpression boundInitializerOpt, AnalyzedArguments analyzedArguments) | ||
| private BoundExpression MakeBadExpressionForObjectCreation(ObjectCreationExpressionSyntax node, TypeSymbol type, AnalyzedArguments analyzedArguments, DiagnosticBag diagnostics) |
There was a problem hiding this comment.
In the case node.Initializer is null the diagnostics parameter won't be used. Should we assert that it's empty in that case?
There was a problem hiding this comment.
I don't know that it will definitely be empty... It gets passed in from much higher up the stack. All we know is that we wouldn't add anything to it, but the code is simple enough that it seems trivially obvious we haven't changed it in the non-initializer case?
jaredpar
reviewed
Oct 29, 2018
|
|
||
| BoundObjectInitializerExpressionBase makeBoundInitializerOpt() | ||
| { | ||
| if (initializerSyntaxOpt != null ) |
|
Close / re-open to trigger CI |
- Add a colleciton initializer test - Fix whitespace nits
333fred
reviewed
Oct 29, 2018
| } | ||
| "; | ||
| var compilation = CreateCompilation(source, options: TestOptions.DebugExe, parseOptions: TestOptions.Regular); | ||
| CompileAndVerify(compilation, expectedOutput: @"1"); |
There was a problem hiding this comment.
Nit: you could just use CompileAndVerify. Not blocking.
There was a problem hiding this comment.
Thanks, this was just copy/paste from the other tests. Cleaned up.
wachulski
added a commit
to wachulski/roslyn
that referenced
this pull request
Oct 31, 2018
…out-if-error-message * origin/master: (1712 commits) User-defined operator checks should ignore differences in tuple member names (dotnet#30774) Attempted fix for correctly reporting error CS1069 when using implicit namespaces (dotnet#30244) Invert the binding order of InitializerExpressions and CreationExpressions (dotnet#30805) Use Arcade bootstrapping scripts (dotnet#30498) Ensure that the compilers produce double.NaN values in IEEE canonical form. (dotnet#30587) Remove properties set in BeforeCommonTargets.targets Fix publishing of dependent projects Contributing page: reference Unix build instructions Delete 0 Propagate values from AbstractProject to VisualStudioProjectCreationInfo Fix publishing nuget info of dev16.0.x-vs-deps branch Revert "Add a SetProperty API for CPS to passing msbuild properties" Validate generic arguments in `using static` directives (dotnet#30737) Correct 15.9 publish data Enable test. Do not inject attribute types into .Net modules. Add a SetProperty API for CPS to passing msbuild properties Revert "add beta2 suffix to dev16 branch" Fix references Remove commit sha from package versions ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #30357